[16.0][MIG] mgmtsystem_nonconformity_project: Migration to 16.0 - #748

👍

LGTM

Code review and test

Automated Review -- Tests Failed

1. Root Cause of the Test Failure

The test failure is caused by a database connection error during Odoo startup (Connection to the database failed). This is likely due to misconfiguration or an environment issue in the test runner, not directly related to the code changes in this PR.

2. Suggested Fix

No code fix is needed for the test failure itself, as it's an infrastructure issue. However, to ensure robustness, the module should be tested with proper database setup. The code changes in mgmtsystem_nonconformity_project/models/mgmtsystem_nonconformity_project.py are valid and follow OCA patterns.

3. Additional Code Issues

There are no real bugs in the code provided. The implementation correctly inherits from mgmtsystem.action, adds a new action_type selection field, and implements a computed field complete_name that respects the action type. The use of _compute_complete_name is appropriate and follows Odoo conventions.

4. Test Improvements

To improve test coverage for this module, consider adding the following test cases in a tests/test_mgmtsystem_nonconformity_project.py file using SavepointCase (as per OCA testing patterns):

from odoo.tests.common import SavepointCase

class TestMgmtsystemNonconformityProject(SavepointCase):
    @classmethod
    def setUpClass(cls):
        super().setUpClass()
        # Create a project
        cls.project = cls.env['project.project'].create({
            'name': 'Test Project',
        })
        # Create a nonconformity action
        cls.action = cls.env['mgmtsystem.action'].create({
            'name': 'Test Action',
            'action_type': 'action',
        })

    def test_action_type_selection(self):
        """Test that action_type field works correctly."""
        self.assertEqual(self.action.action_type, 'action')

    def test_complete_name_action_type(self):
        """Test complete_name computation for action type."""
        self.assertEqual(self.action.complete_name, 'Test Action')

    def test_complete_name_project_type(self):
        """Test complete_name computation for project type."""
        self.action.action_type = 'project'
        self.action.project_id = self.project
        self.assertEqual(self.action.complete_name, 'Test Project')

This test ensures:

• The action_type selection works as expected
• The complete_name field is computed correctly based on action type
• The integration with project.project model works correctly

Use SavepointCase for tests involving data persistence and transactions, as recommended by OCA.

⏰ PR Aging Alert

This PR by @ChanGuaZzz has been open for 164 days (5 months).

Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)

Reciprocal Review Request

Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):

My open PRs across OCA:

• server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
• server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
• hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
• stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
• stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
• stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
• purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix

Reviewing each other's work helps the whole community move forward. Thank you!

Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b

Apologies for the noise from the automated review — the bot incorrectly used "changes requested" status, which revoked the approved label that @cubells had already granted. I'm fixing the bot to use "comment" status instead, so this won't happen again.

Review

Migration looks functionally close to the 12.0 module, and earlier approvals from @dreispt / @cubells still make sense for the core idea. CI is red again after the Apr 7 push that replaced the tests — that needs fixing before merge.

Blocker — CI / tests

The previous green suite used TransactionCase and created the action with required fields (type_action, project_id). The new test file has two concrete problems:

1. Missing required type_action on create — mgmtsystem.action.type_action is required=True, so this will fail:

cls.action = self.env["mgmtsystem.action"].create({
    "name": "Test Action",
    "action_type": "action",
    # type_action missing
})

2. SavepointCase — works on 16.0 only as a deprecated alias of TransactionCase. Prefer TransactionCase (as in the previous test) to avoid DeprecationWarning / future breakage.

Suggested restore/fix (aligned with the old green test):

from odoo.tests import common


class TestNonconformityProject(common.TransactionCase):
    def test_create_action(self):
        action = self.env["mgmtsystem.action"].create(
            {
                "name": "SampleAction",
                "type_action": "immediate",
                "action_type": "project",
                "project_id": self.env.ref("project.project_project_1").id,
            }
        )
        self.assertEqual(action.complete_name, action.project_id.name)

Blocker / bug — computed field without dependencies

complete_name = fields.Char(compute=_compute_complete_name)

Missing @api.depends("name", "action_type", "project_id"), and the compute should be passed as a string. Without depends, test_complete_name_project_type (writes action_type / project_id then asserts complete_name) will not recompute reliably.

@api.depends("name", "action_type", "project_id")
def _compute_complete_name(self):
    for rec in self:
        if rec.action_type == "project" and rec.project_id:
            rec.complete_name = rec.project_id.name
        else:
            rec.complete_name = rec.name

complete_name = fields.Char(compute="_compute_complete_name")

Important — name field redefinition drops required

Parent mgmtsystem.action defines name = fields.Char("Subject", required=True). This module redefines:

name = fields.Char("Claim Subject")

That likely clears required=True. Either keep required=True, or stop redefining the field and only change the label in the view (preferred — “Claim Subject” is legacy wording for an action).

Minor / cleanup (non-blocking)

• Manifest still uses the old OpenERP header; OCA style is a short AGPL copyright header. Consider adding development_status / modern keys if you touch the file again.
• depends includes mgmtsystem_nonconformity while the code only extends mgmtsystem.action — keep it only if there is a real functional link; otherwise mgmtsystem_action + project is enough.
• i18n still uses old selection:mgmtsystem.action,action_type:0 comments; Weblate will refresh after merge.
• View attrs= is fine on 16.0.

Verdict

Request changes — fix CI (tests + @api.depends) and the name required regression, then this should be mergeable again (already has human approvals).